Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add oidc-only flag #213

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add oidc-only flag #213

wants to merge 2 commits into from

Conversation

pavelzw
Copy link

@pavelzw pavelzw commented Sep 9, 2024

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • I used npm run format for formatting the code before submitting the pull request.

Closes #209

Copy link

github-actions bot commented Sep 9, 2024

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@pavelzw
Copy link
Author

pavelzw commented Sep 9, 2024

I have read the CLA Document and I hereby sign the CLA

@pavelzw
Copy link
Author

pavelzw commented Sep 16, 2024

@sverdlov93 could you please take a look at this PR and tell me what you think?

@pavelzw
Copy link
Author

pavelzw commented Sep 30, 2024

@RobiNino could you please take a look?

@EyalDelarea
Copy link
Contributor

Hey @pavelzw,

Thank you for opening this pull request!

I believe a better approach to achieve your goal is to export the OIDC token as an environment variable, which can then be utilized throughout the rest of your workflow.

Make sure to use clear and descriptive names for the variables and ensure they are cleared during the post-action phase to avoid any security risks.

@pavelzw
Copy link
Author

pavelzw commented Jan 15, 2025

Hey @EyalDelarea, thanks for your response!

I believe a better approach to achieve your goal is to export the OIDC token as an environment variable, which can then be utilized throughout the rest of your workflow

As mentioned in #209 (comment), I don't think that exporting the env variables is a good idea. I would prefer to just keep the output of the oidc credentials as it is right now (just using core.setOutput, see below[1])

I could use the oidc outputs in the following way:

- uses: jfrog/setup-jfrog-cli@v4
  id: artifactory
  with:
    oidc-provider-name: ${{ vars.ARTIFACTORY_OIDC_PROVIDER }}
    oidc-audience: ${{ vars.ARTIFACTORY_OIDC_AUDIENCE }}
    oidc-only: true
  env:
    JF_URL: ${{ vars.ARTIFACTORY_URL }}
- uses: prefix-dev/[email protected]
  with:
    auth-host: ${{ vars.ARTIFACTORY_URL }}
    auth-username: ${{ steps.artifactory.outputs.oidc-user }}
    auth-password: ${{ steps.artifactory.outputs.oidc-token }}
# OIDC credentials are not leaked to subsequent steps
- run: some-other-tool

I'm happy to add some additional documentation if you wish.

[1]:

core.setSecret(oidcToken);
// Output the oidc access token as a secret
core.setOutput('oidc-token', oidcToken);
// Output the user from the oidc access token subject as a secret
let payload: JWTTokenData = this.decodeOidcToken(oidcToken);
let tokenUser: string = this.extractTokenUser(payload.sub);
// Mark the user as a secret
core.setSecret(tokenUser);
// Output the user from the oidc access token subject extracted from the last section of the subject
core.setOutput('oidc-user', tokenUser);

@pavelzw
Copy link
Author

pavelzw commented Jan 15, 2025

@EyalDelarea i just rebased on the latest master and fixed all conflicts, let me know what you think!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OIDC-only mode that doesn’t install jfrog CLI
2 participants